Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor link series #1721

Merged
merged 13 commits into from
Nov 20, 2023
Merged

Refactor link series #1721

merged 13 commits into from
Nov 20, 2023

Conversation

payetvin
Copy link
Contributor

@payetvin payetvin self-assigned this Oct 27, 2023
@pull-request-size pull-request-size bot added size/L and removed size/M labels Oct 31, 2023
sylvlecl
sylvlecl previously requested changes Nov 3, 2023
View reviewed changes
Copy link
Member

@sylvlecl sylvlecl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More direct calls to be replaced by wrappers

src/libs/antares/study/area/links.cpp Outdated Show resolved Hide resolved
src/libs/antares/study/area/links.cpp Outdated Show resolved Hide resolved
src/libs/antares/study/area/links.cpp Outdated Show resolved Hide resolved
@payetvin payetvin requested a review from sylvlecl November 6, 2023 14:34
flomnes
flomnes requested changes Nov 13, 2023
View reviewed changes
src/solver/simulation/sim_calcul_economique.cpp Outdated
Comment on lines 566 to 567
ntc.ValeurDeNTCOrigineVersExtremite[k] = lnk.directCapacities.getCoefficient(year, hourInYear);
ntc.ValeurDeNTCExtremiteVersOrigine[k] = lnk.indirectCapacities.getCoefficient(year, hourInYear);
Copy link
Member

@flomnes flomnes Nov 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance improvement :

  1. swap loops on links & hours
  2. use TimeSeries::getColumn a single time instead of TimeSeries::getCoefficient 8700 times

You could do it here, or open a ticket to fix the issue elsewhere as well (I noticed it here and there)

@payetvin what do you think ?

@sonarqubecloud SonarQubeCloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

6.2% 6.2% Coverage
0.0% 0.0% Duplication

@payetvin payetvin merged commit 8fa44ec into develop Nov 20, 2023
7 checks passed
@payetvin payetvin deleted the feature/series-link branch November 20, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a-zakir a-zakir a-zakir left review comments

@flomnes flomnes flomnes approved these changes

@guilpier-code guilpier-code Awaiting requested review from guilpier-code

@sylvlecl sylvlecl Awaiting requested review from sylvlecl

Assignees

@payetvin payetvin

Labels
cleaning code quality size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@payetvin @sylvlecl @flomnes @a-zakir